Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable jetpack app features #15946

Merged
merged 32 commits into from
Feb 18, 2022
Merged

Enable jetpack app features #15946

merged 32 commits into from
Feb 18, 2022

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Feb 11, 2022 •
edited
Loading

Closes #15915
Depends on wordpress-mobile/WordPress-Login-Flow-Android#79

This is a feature branch PR to enable Jetpack app features.

To test:
Please follow the testing steps covered in the linked sub-tasks.

Merging Notes:

  • Wait till referenced login lib PR removing Jetpack app sites filtering is merged to trunk (I will do this merging).
  • Replace wordPressLoginVersion in build.gradle
  • Remove Not Ready for Merge label and merge the PR.

Regression Notes

  1. Potential unintended areas of impact
    Covered in individual sub-tasks.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Covered in individual sub-tasks.

  3. What automated tests I added (or what prevented me from doing so)
    Covered in individual sub-tasks.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

ashiagr added 19 commits February 9, 2022 17:49
@ashiagr
Remove Jetpack filtering for sites
d430673
@ashiagr
Enable add new site menu for Jetpack app
a0f67dc
@ashiagr
Restrict adding self-hosted site in Jetpack app using add new site op…
7ccd749 
…tion
@ashiagr
Update login lib version
85f767f
@ashiagr
Enable add site menu using build config constant
200208f
@ashiagr
Enable new login epilogue with create new site button using build con…
861ad82 
…fig constant
@ashiagr
Restrict adding self-hosted site in Jetpack app using build config co…
4519984 
…nstant
@ashiagr
Add build constant to enable signup in Jetpack app
324f3a0
@ashiagr
Enable signup using build config constant
c7b5e42
@ashiagr
Enable reader using build config constant
4b7206f
@ashiagr
Enable create (page/post/story) FAB using build config constant
2cf853e
@ashiagr
Enable quick action using build config constant
59613d3
@ashiagr
Enable followed sites settings using build config constant
575e55e
@ashiagr
Enable feature announcement using build config constant
aec0761
@ashiagr
Enable App Settings -> What's New using build config constant
45748da
@ashiagr
Enable shortcut for new post
1e19bf6
@ashiagr
Merge pull request #15920 from wordpress-mobile/issue/15916-enable-al…
08c3a64 
…l-sites-jetpack-app

Jetpack App: Show all sites and allow site creation
@ashiagr
Merge pull request #15928 from wordpress-mobile/issue/15922-allow-acc…
9100652 
…ount-creation-jetpack-app

Jetpack app: Enable account creation
@ashiagr
Merge pull request #15930 from wordpress-mobile/issue/15929-enable-mo…
c160deb 
…re-options-jetpack-app

Jetpack App: Enable more options
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 11, 2022 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@ashiagr ashiagr requested a review from zwarm February 11, 2022 14:31
@ashiagr ashiagr self-assigned this Feb 11, 2022
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 11, 2022 •
edited
Loading

You can test the changes on this Pull Request by downloading the APKs:

zwarm
zwarm approved these changes Feb 11, 2022
View reviewed changes
Copy link
Contributor

@zwarm zwarm left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 @ashiagr - I ❤️ the implementation by use of config flags .. it makes it easy to test + add/remove features in the future. Good catch on the shortcuts and feature announcements. And super kudos on the login pieces (very tricky)

I was able to test the following PRs in this feature build and everything worked as expected. I ran through JP and WP.

I'm going to approve, but not merge. I would love to give it one more go after #15944 has been merged in.

Thanks for all the great work!!

@ashiagr
Copy link
Contributor Author

ashiagr commented Feb 14, 2022 •
edited
Loading

Thanks so much @zwarm!

I've merged #15944 and enabled normal login flow if no sites are found (and site creation is enabled) for Jetpack app instead of showing the "No Jetpack sites" screen.
Also, if signup is enabled, Jetpack app will now show a PostSignupInterstitial screen the first time it is shown similar to the WordPress app.

Note: Illustrations on PostSignupInterstitial screen and MySite empty screen are a bit off, I can either
i) hide them or
ii) wait to replace them till we have the new ones. I'll create a task to track it.

EDIT: I've hidden MySite empty screen illustration for Jetpack app as it had WordPress app icon in 1f76982.

Ready for another look. 🙇‍♀️

@ashiagr ashiagr added Tooling and removed Tooling labels Feb 15, 2022
zwarm
zwarm requested changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@zwarm zwarm left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashiagr - I stumbled across a few issues.

  • In Jetpack App: Upon first login to Jetpack app, the illustration in the view appears to be a WP one.

Alt desc

2. In jetpack App: Tapping on Me -> log out doesn't log me out on the first attempt. I have to do it twice. See video.
Screen_Recording_20220216-160110_Jetpack.Pre-Alpha.mp4

@zwarm zwarm assigned zwarm and ashiagr and unassigned ashiagr Feb 16, 2022
@ashiagr ashiagr added Tooling and removed Tooling labels Feb 17, 2022
@ashiagr
Merge branch 'trunk' into feature/15915-enable-jetpack-app-features
1d0949d 
# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteViewModel.kt
#	build.gradle
@ashiagr ashiagr added Tooling and removed Tooling labels Feb 18, 2022
@ashiagr ashiagr mentioned this pull request Feb 18, 2022
Closed
@ashiagr
Copy link
Contributor Author

ashiagr commented Feb 18, 2022

👋 @zwarm,

I've replaced PostSignupInterstitial screen image (f2b93e4, df5022c):

Light Dark
lt_device-2022-02-17-161734 dark_device-2022-02-17-160859

I've also merged login lib PR and updated the corresponding commit hash in this PR. - f552f2e

Regarding the logout (twice click) issue, it is difficult to find out a root cause in a limited time frame, but given that it can be reproduced only when there are no sites for the Jetpack app, and users are able to logout on the next attempt, we can find a fix as part of a separate PR. I've created an issue here to track it.

This is now ready for another look. 🙇‍♀️

@ashiagr ashiagr requested a review from zwarm February 18, 2022 13:11
zwarm
zwarm approved these changes Feb 18, 2022
View reviewed changes
Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced PostSignupInterstitial screen image

I now see the Jetpack illustration when logging in for the first time with an account that has no sites. 👍

Thanks for creating that new PR for the double logout issue. I agree with you, this is definitely an edge case.

I tested the login cases again and all is working as expected. 🙇 Nice job!

@ashiagr ashiagr merged commit f315532 into trunk Feb 18, 2022
@ashiagr ashiagr deleted the feature/15915-enable-jetpack-app-features branch February 18, 2022 14:31
@ashiagr ashiagr mentioned this pull request Mar 1, 2022
Merged
3 tasks
@zwarm zwarm mentioned this pull request May 19, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zwarm zwarm zwarm approved these changes

Assignees

@zwarm zwarm

@ashiagr ashiagr

Labels
None yet
Projects
None yet
Milestone
19.3
Development

Successfully merging this pull request may close these issues.

Changes to Jetpack App
2 participants
@ashiagr @zwarm